-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check if we are on ipv4, ipv6 or dualStack when doing tailscale #7838
Conversation
d9141bf
to
28de47b
Compare
7849673
to
bbd49f6
Compare
bbd49f6
to
3a239e1
Compare
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## master #7838 +/- ##
==========================================
+ Coverage 47.18% 51.45% +4.27%
==========================================
Files 143 143
Lines 14509 14532 +23
==========================================
+ Hits 6846 7478 +632
+ Misses 6576 5865 -711
- Partials 1087 1189 +102
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
pkg/util/net.go
Outdated
@@ -97,10 +97,9 @@ func GetFirst6(elems []net.IP) (net.IP, error) { | |||
// If no IPv6 addresses are found, an error is raised. | |||
func GetFirst6Net(elems []*net.IPNet) (*net.IPNet, error) { | |||
for _, elem := range elems { | |||
if elem == nil || elem.IP.To16() == nil { | |||
continue | |||
if elem != nil || netutils.IsIPv6(elem.IP) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is wrong, you'll now return any non-nil element! The previous code was skipping non-ipv6 addresses; I'm not sure how this is functionally different?
if elem != nil || netutils.IsIPv6(elem.IP) { | |
if elem != nil && netutils.IsIPv6(elem.IP) { |
Also, note that the code you're calling effectively does the same thing - checks for non-nil .To16()
:
https://github.com/kubernetes/utils/blob/9f6742963106fb1fadd7ec2493d6d6cd689e5318/net/ipfamily.go#L106
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good catch! I missed that 🤦
It is slightly different in the ipfamily.go code because by the time to code reaches case ip.To16() != nil:
, ipv4 ips were already returned in the previous line, i.e. only ipv6 addresses can reach case ip.To16() != nil:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Run this gist and you'll see the difference: https://gist.github.com/manuelbuil/73d7e0ff32a69eb045602aeee45896a6
Signed-off-by: Manuel Buil <mbuil@suse.com>
3a239e1
to
f21a014
Compare
Proposed Changes
Instead of listing the IPs of the tailscale interface in the variable
IPs []net.IP
, separate the ipv4 and ipv6 address that are always assigned to the tailscale interface:That way we can easily control what address to use in kube-api depending on the configured mode in k3s.
Also a bug in utils/net.go was fixed:
is not a correct way to check if an IP is ipv6. It works for both ipv4 and ipv6!
Types of Changes
Bugfix
Verification
Verify that this works in:
Deploy k3s in three modes:
And check what IP is used for kubeapi-server in the flag
advertised-ip
. It should be ipv4 for ipv4-only and dual-stack mode. Whereas it should be ipv6 for ipv6-only mode.Testing
Linked Issues
#7831
User-Facing Change
Further Comments